Skip to content

Migrate flyteconnector chart wiring and runtime defaults for Flyte Connectors#7377

Draft
kevinliao852 wants to merge 8 commits into
flyteorg:mainfrom
kevinliao852:feature/migrate-flyteconnector
Draft

Migrate flyteconnector chart wiring and runtime defaults for Flyte Connectors#7377
kevinliao852 wants to merge 8 commits into
flyteorg:mainfrom
kevinliao852:feature/migrate-flyteconnector

Conversation

@kevinliao852
Copy link
Copy Markdown
Contributor

@kevinliao852 kevinliao852 commented May 15, 2026

Tracking issue

Why are the changes needed?

This PR completes the first pass of the flyteconnector chart migration to the new Flyte Connectors image/runtime.

What changes were proposed in this pull request?

  • Wire flyteconnector into flyte-binary as a real Helm dependency
    • Vendor the subchart under charts/flyte-binary/charts/
    • Update connector image repository from cr.flyte.org/flyteorg/flyteagent to ghcr.io/flyteorg/flyte-connectors
    • Change the default connector image tag to latest
    • Fix additionalVolumeMounts so extra mounts render under volumeMounts instead of breaking the Deployment YAML
    • Remove showcase secret data from default values by making connectorSecret.secretData empty
    • Update the connector startup command from the stale
      pyflyte serve agent to the verified runtime command flyte serve connector

How was this patch tested?

Labels

Please add one or more of the following labels to categorize your PR:

  • added: For new features.
  • changed: For changes in existing functionality.
  • deprecated: For soon-to-be-removed features.
  • removed: For features being removed.
  • fixed: For any bug fixed.
  • security: In case of vulnerabilities

This is important to improve the readability of release notes.

Setup process

Screenshots

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

Stack

If you do use git town to manage PR Stacks, the stack relevant to this PR
will show below. Otherwise, you can ignore this section.

Docs link

Signed-off-by: Kevin Liao <q85292542000@gmail.com>
- Added `flyteconnector` as a dependency in `Chart.yaml`.
- Updated image repository and tag in `flyteconnector` README and `values.yaml` to use the latest version from GitHub.

Signed-off-by: Kevin Liao <q85292542000@gmail.com>
…efaults

Signed-off-by: Kevin Liao <q85292542000@gmail.com>
Signed-off-by: Kevin Liao <q85292542000@gmail.com>
Copilot AI review requested due to automatic review settings May 15, 2026 14:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the flyteconnector Helm chart to the new Flyte Connectors runtime and wires it into flyte-binary as a real Helm subchart dependency. It updates the connector image to ghcr.io/flyteorg/flyte-connectors:latest, switches the container command to flyte serve connector, fixes additionalVolumeMounts rendering, and clears the showcase secret data from the chart defaults.

Changes:

  • Add flyteconnector as a file://-based subchart dependency of flyte-binary (gated by flyteconnector.enabled).
  • Update connector image, tag, and startup command in the chart's deployment template and defaults.
  • Empty out connectorSecret.secretData and adjust template indentation so additional volume mounts render correctly.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
charts/flyte-binary/Chart.yaml Adds the flyteconnector subchart as a conditional Helm dependency.
charts/flyteconnector/Chart.yaml New chart manifest (v0.1.10).
charts/flyteconnector/.helmignore Standard helm ignore patterns for the new chart.
charts/flyteconnector/README.md Auto-generated values reference for the new chart.
charts/flyteconnector/values.yaml New default values: image, ports, RBAC, probes, service, etc.
charts/flyteconnector/templates/_helpers.tpl Helper templates for names, labels, secret volume/mount, service port.
charts/flyteconnector/templates/connector/deployment.yaml Connector Deployment with flyte serve connector command and fixed volume-mount rendering.
charts/flyteconnector/templates/connector/service.yaml gRPC ClusterIP Service for the connector.
charts/flyteconnector/templates/connector/serviceaccount.yaml Optional ServiceAccount with annotations and imagePullSecrets.
charts/flyteconnector/templates/connector/secret.yaml Opaque Secret pseudo-manifest fed by connectorSecret.secretData.
charts/flyteconnector/templates/connector/rbac.yaml Optional ClusterRole/ClusterRoleBinding for cross-namespace secret reads.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread charts/flyteconnector/values.yaml Outdated
Comment thread charts/flyteconnector/values.yaml Outdated
{{- end }}

{{- define "flyteconnector.servicePort" -}}
{{ include .Values.ports.containerPort}}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copilot might be right

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I've made the requested changes

Comment on lines +40 to +41
# -- Default regex string for searching configuration files
configPath: /etc/flyteconnector/config/*.yaml
Comment on lines +9 to +12
# commonLabels Add labels to all the deployed resources
commonLabels: {}
# commonAnnotations Add annotations to all the deployed resources
commonAnnotations: {}
# -- Docker image for flyteconnector deployment
repository: ghcr.io/flyteorg/flyte-connectors # FLYTECONNECTOR_IMAGE
# -- Docker image tag
tag: latest # FLYTECONNECTOR_TAG
Comment on lines +1 to +7
apiVersion: v1
kind: Secret
metadata:
name: {{ template "flyteconnector.name" . }}
namespace: {{ template "flyte.namespace" . }}
type: Opaque
{{- with .Values.connectorSecret.secretData -}}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we update here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I updated this by making the Secret conditional on connectorSecret.secretData.
When secretData is empty, the chart no longer renders an empty Opaque Secret.

Comment on lines +11 to +15
{{- end}}
{{- with .Values.serviceAccount.imagePullSecrets }}
imagePullSecrets: {{ tpl (toYaml .) $ | nindent 2 }}
{{- end }}
{{- end }}
Comment on lines +1 to +7
apiVersion: v1
kind: Secret
metadata:
name: {{ template "flyteconnector.name" . }}
namespace: {{ template "flyte.namespace" . }}
type: Opaque
{{- with .Values.connectorSecret.secretData -}}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we update here?

{{- end }}

{{- define "flyteconnector.servicePort" -}}
{{ include .Values.ports.containerPort}}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copilot might be right

Comment thread charts/flyteconnector/Chart.yaml Outdated
name: flyteconnector
description: A Helm chart for Flyte connector
type: application
version: v0.1.10 # VERSION
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
version: v0.1.10 # VERSION
version: v2.0.0 # VERSION

# -- Docker image for flyteconnector deployment
repository: ghcr.io/flyteorg/flyte-connectors # FLYTECONNECTOR_IMAGE
# -- Docker image tag
tag: latest # FLYTECONNECTOR_TAG
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use 2.3.6 for now. ghcr.io/flyteorg/flyte-connectors:py3.12-v2.3.6

Comment thread charts/flyte-binary/Chart.yaml Outdated

dependencies:
- name: flyteconnector
version: v0.1.10
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
version: v0.1.10
version: v2.0.0

Copilot AI review requested due to automatic review settings May 29, 2026 22:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.

Comment on lines +16 to +18
{{- define "flyteconnector.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}}
{{- end -}}
Comment on lines +38 to +44
{{- if or .Values.connectorSecret.secretData .Values.additionalVolumeMounts }}
volumeMounts:
{{- include "connectorSecret.volumeMount" . | nindent 8 }}
{{- with .Values.additionalVolumeMounts -}}
{{ tpl (toYaml .) $ | nindent 8 }}
{{- end }}
{{- end }}
Comment on lines +58 to +64
serviceAccountName: {{ template "flyteconnector.name" . }}
{{- if or .Values.connectorSecret.secretData .Values.additionalVolumes }}
volumes: {{- include "connectorSecret.volume" . | nindent 6 }}
{{- with .Values.additionalVolumes -}}
{{ tpl (toYaml .) $ | nindent 6 }}
{{- end }}
{{- end }}
Comment on lines +13 to +16
annotations:
{{- with .Values.podAnnotations }}
{{- toYaml . | nindent 8 }}
{{- end }}
Comment on lines +48 to +51
readinessProbe:
{{- with .Values.readinessProbe -}}
{{ tpl (toYaml .) $ | nindent 10 }}
{{- end }}
Comment on lines +3 to +13
{{- define "flyte.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}}
{{- end -}}

{{- define "flyte.chart" -}}
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" -}}
{{- end -}}

{{- define "flyte.namespace" -}}
{{- default .Release.Namespace .Values.forceNamespace | trunc 63 | trimSuffix "-" -}}
{{- end -}}
name: flyteconnector
description: A Helm chart for Flyte connector
type: application
version: v2.0.0 # VERSION
Comment on lines +14 to +18
ports:
- name: {{ .Values.ports.name }}
port: {{ .Values.ports.containerPort }}
protocol: TCP
appProtocol: TCP
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants